⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Known Issues

Tests intentionally excluded from CI. Source of truth for the Known
Issues section the L1 workflow appends to each ef-tests job summary
and posts as a sticky PR comment.

EF Tests — Stateless coverage narrowed to EIP-8025 optional-proofs

make -C tooling/ef_tests/blockchain test calls test-stateless-zkevm
instead of test-stateless. The zkevm@v0.3.3 fixtures are filled against
bal@v5.6.1, out of sync with current bal spec; the broad target trips ~549
fixtures. Re-broaden once the zkevm bundle is regenerated.

🤖 Kimi Code Review

This PR fixes an interoperability-critical encoding bug in EIP-8159 Block Access Lists (BAL) handling. The change is correct and well-tested.

Summary

Issue: The previous implementation used RLP empty list (0xc0) as the sentinel for "BAL unavailable", which aliases with a valid empty BAL (block with no state changes). This breaks consensus with geth and other clients that use 0x80 (RLP empty string) per EIP-8159.

Fix: Correctly uses 0x80 as the sentinel value for None.

Detailed Review

crates/networking/p2p/rlpx/eth/block_access_lists.rs

Lines 28-31, 42-45: The encoding/decoding logic correctly distinguishes between:

• 0x80 (empty string): Sentinel for "BAL unavailable"
• 0xc0 (empty list): Valid encoding of empty BAL (no state changes)

This prevents silent ambiguity where a node couldn't distinguish between "this block has no state changes" vs "this block's BAL is unknown".

Line 47: The decode logic properly falls through to BlockAccessList::decode_unfinished for non-sentinel values. Consider adding an explicit check for empty input to provide a clearer error message, though the current implementation is functionally correct:

// Optional enhancement for clearer errors:
if rlp.is_empty() {
    return Err(RLPDecodeError::MalformedData);
}

Lines 252-277: The test block_access_lists_none_uses_0x80_sentinel is excellent:

• Verifies the sentinel byte is present in the wire format
• Verifies the old incorrect sentinel is absent
• Tests roundtrip encoding/decoding
• Documents the geth interoperability requirement

Line 252: The import use crate::rlpx::utils::snappy_decompress; is appropriate for testing the actual wire format (RLP + snappy compression).

Verdict

Approve. This is a precise fix for a protocol interoperability bug. The documentation comments accurately reference EIP-8159 and explain why 0x80 is the only valid sentinel (empty string cannot alias a valid BAL encoding, while empty list can).

Action required: Ensure this change is included in any release notes as it fixes compatibility with geth's EIP-8159 implementation.

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

🤖 Claude Code Review

Code Review: PR #6744 — Fix eth/71 0x80 sentinel for missing BALs

Summary

Correct, well-scoped fix. The sentinel byte change from 0xc0 to 0x80 is spec-compliant per EIP-8159 and matches go-ethereum's implementation. The bug was a genuine protocol interop issue with real consequences, and the PR description explains it clearly.

Findings

1. Core fix is correct

crates/networking/p2p/rlpx/eth/block_access_lists.rs:31, 37, 45 — The encode/decode changes are minimal and correct:

• 0x80 (RLP empty string) cannot alias a real BAL since a BlockAccessList is always encoded as a list starting with 0xc0+n or 0xf8+n.
• The length() function correctly still returns 1 for None since both bytes are single-byte values.
• On decode, 0x80 is unambiguous: no valid list type starts with it.

2. Test assertion contains(&0x80) is vacuously true

block_access_lists.rs:268-270

assert!(
    decompressed.contains(&0x80),
    "decompressed payload must contain the 0x80 None sentinel"
);

request_id = 0 encodes in RLP as 0x80 (integer 0 is the empty byte string). This assertion will pass even if the None sentinel is broken. The roundtrip is_none() check at line 279–281 is doing the real work here, so this isn't a bug—but the contains(&0x80) line provides no additional confidence beyond what the roundtrip check already proves.

Consider using a non-zero request_id (e.g. 1) so the 0x80 presence check only fires when the sentinel itself is present. Or drop that assertion and rely solely on the roundtrip.

3. Test assertion !contains(&0xc0) is correct but fragile

block_access_lists.rs:271-274

The assertion holds for this particular payload: the outer list is non-empty (producing 0xc1+ prefixes, not 0xc0), and no other sub-structure produces a bare empty list. However, if a future change to BlockAccessLists encoding introduces a legitimately empty inner list, this check will fail spuriously. The value here is as a regression guard against reverting the sentinel, which is worth keeping—just worth noting the brittleness.

4. Backward compatibility of the decode path

After this fix, an old ethrex peer sending 0xc0 will no longer be recognized as None; instead it falls through to BlockAccessList::decode_unfinished, which will decode it as an empty (valid) BAL. The PR description acknowledges this as intentional, and it's the correct choice—the spec says 0x80 and we shouldn't be lenient with a buggy sentinel that causes silent data confusion.

Minor Nits

• The doc comment on OptionalBal (struct definition, lines 16–22) now says both None → 0x80 sentinel and Some(bal) → normal RLP list, which is accurate and reads well.
• No trailing comment needed on length() line 37; // empty string = 0x80 per EIP-8159 is fine as a spec citation.

Verdict

Approve with minor suggestions. The fix is correct, spec-compliant, and the test is good enough with the roundtrip assertion anchoring actual behavior. The contains(&0x80) assertion is cosmetically redundant; either swap to request_id = 1 or remove it. Nothing blocks merging.

Automated review by Claude (Anthropic) · sonnet · custom prompt

Lines of code report

Total lines added: 6
Total lines removed: 0
Total lines changed: 6

+-------------------------------------------------------------+-------+------+
| File                                                        | Lines | Diff |
+-------------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/block_access_lists.rs | 211   | +6   |
+-------------------------------------------------------------+-------+------+

Greptile Summary

Fixes the OptionalBal sentinel byte in the eth/71 BlockAccessLists (0x13) response handler, switching from 0xc0 (RLP empty list) to 0x80 (RLP empty string) per EIP-8159, matching go-ethereum's rlp.EmptyString sentinel. The old byte caused silent data confusion with geth because 0xc0 is a valid encoding for an empty (but present) BAL.

• Encode/decode changed: OptionalBal::encode now emits 0x80 for None; decode_unfinished now recognises 0x80 (not 0xc0) as the missing-BAL signal.
• New test added: block_access_lists_none_uses_0x80_sentinel decompresses the wire payload and verifies 0x80 is present and 0xc0 absent, then roundtrips to confirm None is preserved end-to-end.

Confidence Score: 5/5

Safe to merge — a single, surgical byte-constant change with matching encode/decode symmetry and a new wire-level regression test.

The change is minimal: two byte literals flipped from 0xc0 to 0x80 with a matching decode guard. The fix aligns with both the EIP-8159 spec and geth's reference implementation. All pre-existing roundtrip tests continue to pass because both sides of the codec were updated together. The new test independently checks the decompressed wire bytes, ensuring the sentinel cannot silently regress.

No files require special attention.

Sequence Diagram

sequenceDiagram
    participant ethrex as ethrex (eth/71)
    participant wire as Wire (RLP)
    participant geth as go-ethereum (eth/71)

    Note over ethrex,geth: BlockAccessLists (0x13) response — missing BAL slot

    ethrex->>wire: OptionalBal(None) → encode 0x80 (RLP empty string, EIP-8159)
    wire->>geth: byte 0x80
    geth->>geth: "rlp.EmptyString check passes → slot = unavailable ✓"

    Note over ethrex,geth: Before fix (wrong sentinel 0xc0)
    ethrex-->>wire: OptionalBal(None) → encode 0xc0 (RLP empty list)
    wire-->>geth: byte 0xc0
    geth-->>geth: "decode as zero-element list → slot = valid empty BAL ✗ (silent confusion)"

_{Reviews (1): Last reviewed commit: "fix(l1): use 0x80 sentinel for missing e..." | Re-trigger Greptile}

🤖 Codex Code Review

1. Medium: block_access_lists.rs changes the decoder to recognize only 0x80 as None, but a pre-fix Ethrex peer that still sends legacy 0xc0 for “BAL unavailable” will now decode as Some(empty_bal) because empty BAL is valid RLP. That is not a harmless difference: the sync path forwards Some(...) into block execution (full.rs), which takes the Amsterdam header-BAL validation path (mod.rs, mod.rs) and can reject an otherwise valid block instead of falling back to “no BAL available”. If mixed-version peer compatibility matters, this needs an explicit fallback strategy higher up the stack; otherwise upgraded nodes can false-fail sync against older Ethrex peers.

2. Low: block_access_lists.rs does not actually prove the BAL field uses the 0x80 sentinel. With id = 0, the payload already contains an unrelated 0x80 for the request id, so decompressed.contains(&0x80) passes even if the BAL item regresses to some other non-0xc0 encoding. !contains(&0xc0) is also brittle if the payload shape changes. This would be stronger if it asserted the exact second field encoding or exact decompressed bytes for this minimal case.

The wire-format fix itself looks correct and aligns with the spec; the concerns above are about backward compatibility and test precision.

I could not run cargo test here because rustup tried to write under /home/runner/.rustup/tmp on a read-only filesystem.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt